Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds tenant-level ban/unban functionality to Supavisor, including an API endpoint for toggling the ban state and a connection-time check that rejects banned tenants with a wire-level FATAL error.
Changes:
- Adds
PATCH /api/tenants/:external_idto ban/unban tenants via OpenAPI-cast body params. - Persists ban state via new
tenants.banned_at/tenants.ban_reasoncolumns and updates tenant/domain logic accordingly. - Enforces bans during connection establishment and adds unit/integration test coverage for the new behavior.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| test/supavisor_web/controllers/tenant_controller_test.exs | Adds controller tests for PATCH ban/unban behavior, response fields, and cache invalidation. |
| test/supavisor/client_handler_test.exs | Adds unit tests for the new ban check and wire error formatting. |
| test/integration/proxy_test.exs | Adds integration coverage asserting banned tenants receive a wire-level FATAL error and can connect after unban. |
| priv/repo/migrations/20260323000000_add_tenant_ban_fields.exs | Adds nullable banned_at and ban_reason columns to tenants table. |
| lib/supavisor_web/router.ex | Adds PATCH route and makes ApiSpec available in the :api pipeline. |
| lib/supavisor_web/open_api_schemas.ex | Adds ToggleTenantBan request schema for PATCH endpoint. |
| lib/supavisor_web/controllers/tenant_controller.ex | Implements PATCH action using CastAndValidate + Tenants.toggle_tenant_ban/2. |
| lib/supavisor/tenants/tenant.ex | Adds schema fields and ban/unban changesets. |
| lib/supavisor/tenants.ex | Adds toggle_tenant_ban/2 public API with cache invalidation. |
| lib/supavisor/errors/tenant_banned_error.ex | Introduces TenantBannedError (EBANNED) used for wire-level rejection. |
| lib/supavisor/client_handler/checks.ex | Adds check_tenant_not_banned/1 and wires it into connection checks. |
| lib/supavisor/client_handler.ex | Calls the new ban check before completing connection establishment. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mentels
commented
Mar 24, 2026
| } | ||
|
|
||
| # TODO: fix the other actions to use CastAndValidate and remove this conditional plug | ||
| plug OpenApiSpex.Plug.CastAndValidate, [json_render_error_v2: true] when action == :patch |
Contributor
Author
There was a problem hiding this comment.
I think it is useful as this does API params validation for us.
It also facilitates testing as we can assert on schemas etc.
mentels
commented
Mar 24, 2026
| } | ||
| ) | ||
|
|
||
| def patch(conn, %{external_id: id}) when is_boolean(conn.body_params.banned) do |
Contributor
Author
There was a problem hiding this comment.
I'm not happy with the action name but I couldn't come up with anything better than patch; I think update would be better here as this is what the PATCH is meant for but it's already taken.
I envision this action to be used for other tenant modifications too.
mentels
commented
Mar 24, 2026
mentels
force-pushed
the
chore/add-api-for-banning-unbanningt-tenant
branch
3 times, most recently
from
b17942e to
f633643
Compare
Introduces a PATCH /api/tenants/:external_id endpoint for banning and unbanning tenants. Banned tenants receive a FATAL wire-level error (EBANNED) on any new connection attempt. Changes: - Add PATCH route and patch/2 controller action backed by Tenants.toggle_tenant_ban/2 - Add banned_at / ban_reason fields to the tenants table via migration - Add TenantBannedError and wire it into ClientHandler.Checks so connections to banned tenants are rejected at the check stage - Add ToggleTenantBan OpenAPI schema and register PutApiSpec in the :api pipeline; scope CastAndValidate to :patch only (other actions have a TODO for a follow-up PR) - Add controller, unit, and integration tests covering ban/unban, boolean-type validation, 404 handling, and cache invalidation Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
mentels
force-pushed
the
chore/add-api-for-banning-unbanningt-tenant
branch
from
5030654 to
ea91d69
Compare
mentels
force-pushed
the
chore/add-api-for-banning-unbanningt-tenant
branch
from
4e802f7 to
ac5fec9
Compare
v0idpwn
reviewed
Mar 25, 2026
Member
v0idpwn
left a comment
v0idpwn
left a comment
There was a problem hiding this comment.
Code looks good, but check should be ok with expired bans
check_tenant_not_banned/1 now allows connections when banned_until is set and already in the past (time-limited ban expired). Adds tests for permanent ban, future expiry, and past expiry cases. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PATCH /api/tenants/:external_idfor banning and unbanning tenants. Banned tenants receive aFATALwire-level error (EBANNED) on any new connection attempt.banned_at/ban_reasonfields to thetenantstable via migration, and aTenantBannedErrorthat is checked inClientHandler.Checksbefore a connection is established.https://linear.app/supabase/issue/POOLER-275/support-banning-and-unbanning-tenants-via-http-api
OAS / CastAndValidate scope
OpenApiSpex.Plug.CastAndValidateis scoped to the:patchaction only via a conditional plug.PutApiSpecis added to the:apipipeline so the spec is available for casting. ExtendingCastAndValidateto the remaining actions is left as a TODO for a follow-up PR — those actions currently pass params as string-keyed maps and would require coordinated changes.TODO